Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

include cmake rules for third party libraries only if it needed #66

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

vitlav
Copy link
Contributor

@vitlav vitlav commented Jul 4, 2021

Ever when TG_OWT_PACKAGED_BUILD is used, the cmake rules for embedded (third party) sources are still included and they access the third party sources.
This patch fixed that incorrect behaviour and allows build with system libraries without third party sources using.

@vitlav vitlav force-pushed the fix-load-external branch from ad0dbef to ad47b06 Compare July 5, 2021 00:33
@@ -117,6 +117,9 @@ function(link_libabsl target_name)
endif()
endif()
if (NOT absl_FOUND)
if (NOT TARGET libabsl)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why other cases don't do this check?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only link_libabsl called twice. Will I add this check to other cases?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In theory any of them could be called any amount of times

@vitlav vitlav force-pushed the fix-load-external branch from ad47b06 to b588aee Compare July 9, 2021 15:15
@ilya-fedin
Copy link
Contributor

Hey, I finally tested your PR on CI. Unfortunately, it still makes snap fail, just like your previous PRs. You should find another way to do that.

https://github.com/ilya-fedin/tdesktop/runs/3626654353?check_suite_focus=true

@ilya-fedin
Copy link
Contributor

Windows CI failed as well
https://github.com/ilya-fedin/tdesktop/runs/3626654319?check_suite_focus=true

MacOS CI is broken at the moment, but I guess it would fail as well

@ilya-fedin
Copy link
Contributor

@vitlav will there be any further work or the PR can be closed?

@vitlav
Copy link
Contributor Author

vitlav commented Sep 22, 2021

@vitlav will there be any further work or the PR can be closed?

I still can't open the CI logs :(
And I will have a time for a further work only in a month...

@ilya-fedin
Copy link
Contributor

I still can't open the CI logs :(

Why can't you open them?

@vitlav
Copy link
Contributor Author

vitlav commented Sep 22, 2021

I still can't open the CI logs :(

Why can't you open them?

image
I will try later.

@ilya-fedin
Copy link
Contributor

I will try later.

Maybe you have JS disabled or some extensions like uMatrix? It uses AJAX to fetch the logs. You can click on the gear and download log archive.

@ilya-fedin
Copy link
Contributor

@vitlav are you still busy? looks like you got some merge conflicts

@klemensn
Copy link
Contributor

Hey, I finally tested your PR on CI. Unfortunately, it still makes snap fail, just like your previous PRs. You should find another way to do that.

https://github.com/ilya-fedin/tdesktop/runs/3626654353?check_suite_focus=true

Perhaps #85 is "another way"?

@ilya-fedin
Copy link
Contributor

ilya-fedin commented Feb 16, 2022

@vitlav there were no work for more than half of a year (github says 7 months), maybe close the PR and re-open when you would have time to work on this?

@vitlav
Copy link
Contributor Author

vitlav commented Feb 16, 2022

@vitlav there were no work for more than half of a year (github says 7 months), maybe close the PR and re-open when you would have time to work on this?

PR #85 is really "another way", so I will close this PR.

@ilya-fedin
Copy link
Contributor

#85 has the same problems as your PR

@ilya-fedin
Copy link
Contributor

There are conflicts now

@klemensn
Copy link
Contributor

There are conflicts now

I think this PR can be closed as stale and duplicate of #85.

@ilya-fedin
Copy link
Contributor

I was unable to pursue preston to close it :(
As well as other abandoned ones...
The only way out is if the author will close it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants